Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Waf update #421

Merged
merged 28 commits into from
Aug 25, 2019
Merged

Waf update #421

merged 28 commits into from
Aug 25, 2019

Conversation

ederag
Copy link
Collaborator

@ederag ederag commented Jul 11, 2019

This is a PR for the waf update (issue #399).

Need to (at least):

  • add a license file to the waflib directory
  • restore icon cache updates
  • new waf dbus needs dbus-binding-tool ?
  • installation instructions: documentation needs yelp
  • restore help build
  • make help installation optional (wip + need doc)
  • update scrollkeeper ?
  • change default prefix to usr
  • do not use conf.define (get rid of compat15)
  • more tests + instructions for other distros
  • add uninstall instructions
  • update NEWS

ederag added 8 commits June 29, 2019 20:55
curl -o waf https://waf.io/waf-2.0.17
waflib from waf commit 190555a8d631b387323e7d7d3771c56fa1884743
some dependencies (e.g. intltool) might be installed only for python3
data/wscript_build:
- removed misleading #!/usr/bin/env python2
- wildcards need ant_glob now

wscript:
- help is excluded for now.
- gnome feature is no longer available in waf.
  => postinstall_icons and postinstall_scrollkeeper commented out for now.
- hamster-time-tracker.schemas is first generated in the build root,
  and subsequently installed to /etc/gconf/schemas/.
  The destination can be changed with the new --gconf-dir option.
  So packagers would use something like
  ./waf configure build --gconf-dir=%{_sysconfdir}/gconf <...>
- gconf schemas are now correctly registered globally with
  --makefile-install-rule
  this should reduce the risk of facing issue 265 again
  ("Tracking settings don't save")
- bld.new_task_gen(...) is not available any longer,
  replaced with bld(...)
License copied on 2019-07-13 from
https://waf.io/apidocs/copyright.html
Same as the main waf script license header.
Checked that this is the New BSD License (3-clause)
https://en.wikipedia.org/wiki/BSD_licenses#3-clause
as also stated in
https://en.wikipedia.org/wiki/Waf#License
manage_gconf_schemas and update_icon_cache
otherwise appeared as valid commands in ./waf --help.
Building and installing help pages now work with the new waf version.
WIP: doc_commands is not fully exploited yet
@ederag
Copy link
Collaborator Author

ederag commented Jul 27, 2019

Docs and main app have been separated.
hamster works fine on ubuntu-19.04. The help should be available.
Please note that installation instructions changed.

I'm giving up "allow to build documentation as static html files",
not obvious, and not worth the trouble for now.

scrollkeeper is no longer used.

@ederag
Copy link
Collaborator Author

ederag commented Aug 5, 2019

Installation worked on clean openSUSE Leap 15.1, ubuntu 18.04 and 19.04 virtualboxes, but
66baf83 looks like a hack.
Moreover splitting the documentation and main app builds brings trouble for the uninstall process.
Let us just skip the documentation build if xml2po is not found, to reduce dependencies.

ederag added 2 commits August 5, 2019 23:04
Just skip documentation if xml2po is not found.
@ederag
Copy link
Collaborator Author

ederag commented Aug 6, 2019

Done.
Tested on clean openSUSE Leap 15.1, ubuntu 18.04 and 19.04 virtualboxes.

Feedback would be welcome !

For review, here are the changes except the waf update itself.

@ederag ederag changed the title [WIP] Waf update Waf update Aug 6, 2019
@ederag ederag added this to the v2.3 milestone Aug 6, 2019
@GeraldJansen
Copy link
Contributor

GeraldJansen commented Aug 10, 2019

Feedback would be welcome !

Works for me on Xubuntu 19.04! Including the help pages. Many thanks for all that work!

The only glitch I ran into is that I normally use umask 0007 and that caused the files installed in /usr to be not readable by regular users. After uninstalling, and starting over with umask 0002 all went smoothly. (Maybe waf could set the umask to avoid this issue.)

Maybe there is not much feedback because some people (like me) don't know how to go about testing a pull request. This is the way I did it:
git clone --branch=waf-update [email protected]:ederag/hamster.git hamster-waf-update
but there are probably better ways (git add remote?).

Before installing I did a sudo ./waf uninstall from my previous version and sudo rm -rf /usr/lib/python3/dist-packages/hamster. I also found a few other leftover files with find /usr -name "hamster*" and removed those too.

@ederag
Copy link
Collaborator Author

ederag commented Aug 10, 2019

Thanks for the feedback !

Maybe waf could set the umask to avoid this issue.

Forcing umask might be disturbing.
pip use standard umask, and so does waf.
A warning would be nice, but how to get the default umask ?

[...] there are probably better ways (git add remote?).

Thanks for the "one-shot" command !
Wiki updated with more complete instructions.

I also found a few other leftover files with find /usr -name "hamster*"

That should not happen in principle. Here is a guess.
To correctly remove a version earlier than this PR (e.g. v2.2.2),
the --prefix=/usr is required for the configure step:

git checkout v2.2.2
./waf configure --prefix=/usr
sudo ./waf uninstall

@GeraldJansen
Copy link
Contributor

Thanks for the tip about umask and sudo pip install: finally I know why I've always had those permission problems installing with pip! For hamster, maybe just change the docs to ( umask 0022 && sudo ./waf install; ). Works for me and leaves my usual umask unaltered.

My "hamster*" left-overs in /usr may have come from some manual interventions trying to get the docs/yelp working with xfce in various past versions. Uninstall works cleanly now.

Thanks also for the improvements in the documentation about Contributing. Super.

@ederag
Copy link
Collaborator Author

ederag commented Aug 10, 2019

( umask 0022 && sudo ./waf install; ). Works for me and leaves my usual umask unaltered.

That is quite surprising, as sudo is designed to be insensitive to the calling environment.
Indeed here,

> ( umask 0002 && sudo bash -c umask; )
0022

Trying to understand, I found this very interesting answer about default umask in ubuntu.

@GeraldJansen
Copy link
Contributor

GeraldJansen commented Aug 10, 2019

Based on this, sudo usually is sensitive to the user's umask. My solution is not very elegant but it protects against an overly restrictive umask. Now I've added these lines to /etc/sudoers so I personally won't have the problem anymore:
Defaults umask=0022
Defaults umask_override

@ederag
Copy link
Collaborator Author

ederag commented Aug 11, 2019

Thanks for the link, this makes sense now.
Something has to be done. I would add some explanations,
otherwise that might look suspicious to people who never faced the issue.
For instance:
"""

# thanks to the parentheses the umask of your shell will not be changed
( umask 0022 && sudo ./waf install; )

The umask 0022 is safe for all, but important for users with more restrictive umask,
as discussed here.
"""

Finally, it happens that the instructions are not exactly copy/pasted anyway.
So to help finding this discussion, in case of a too restrictive umask, launching hamster yields

Traceback (most recent call last):
  File "/usr/bin/hamster", line 30, in <module>
    from hamster import client, reports
ImportError: cannot import name 'client' from 'hamster' (unknown location)

@ederag ederag merged commit d15555e into projecthamster:master Aug 25, 2019
@ederag ederag deleted the waf-update branch August 25, 2019 07:19
@ederag
Copy link
Collaborator Author

ederag commented Aug 25, 2019

Thanks again for the feedback Gerald !

@GeraldJansen
Copy link
Contributor

A big step. Yahoo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants